Skip to content

Fix #18337 - Emit coverage counters at call sites of inlined functions#22922

Open
suyashkumar102 wants to merge 15 commits intodlang:masterfrom
suyashkumar102:fix/cov-inline-final
Open

Fix #18337 - Emit coverage counters at call sites of inlined functions#22922
suyashkumar102 wants to merge 15 commits intodlang:masterfrom
suyashkumar102:fix/cov-inline-final

Conversation

@suyashkumar102
Copy link
Copy Markdown
Contributor

Fixes #18337

Ran into this while working on #22911. @nordlow brought it up, and since I was already around the inliner code, I took a look.

Tracing through it: expandInline replaces the call with a CommaExp chain
of the inlined body, but the standalone function body is what gets the coverage
counters — but it doesn't execute at runtime. The inlined copy inside the caller
had nothing.

Following @dkorpel's suggestion in #22830, expandInline now tags those
CommaExp chains with isInlineSequence. In visitComma (e2ir.d), tagged
chains get incUsageElem for each sub-expression with a source location from
the inlined body. Parameter copies use Loc.initial to avoid inflating counts
on the declaration line. I’m not entirely sure if that’s the cleanest solution, but it works.

Tested on Windows with the patched compiler :

Before and after on test_cov.d:

Before :

a1

After :

a2

Same fix applies with -inline flag, both paths go through expandInline:

c

Counter fires correctly for nested inlining too — inner() called 3 times shows 3:

b

Normal non-inlined coverage unchanged — never_called() still shows 0000000:

d

inlined functions always showed 0000000 in -cov reports even when called.
the standalone function body is never executed at runtime (it's inlined at
each call site), so its counters stayed zero.

fix: tag the CommaExp chains that expandInline produces with isInlineSequence,
then emit coverage counters for those chains in visitComma (e2ir.d).

fixes https://issues.dlang.org/show_bug.cgi?id=5848
fixes dlang#18337
@dlang-bot
Copy link
Copy Markdown
Contributor

Thanks for your pull request and interest in making D better, @suyashkumar102! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#22922"

Comment thread compiler/src/dmd/glue/e2ir.d Outdated
parameter copies in expandInline were using vfrom.loc, pointing to the declaration line and inflating its count. using Loc.initial instead so inlineCoverage skips them - only actual body statements get counted.
Comment thread compiler/src/dmd/glue/e2ir.d Outdated
Comment thread compiler/src/dmd/frontend.h
suyashkumar102 and others added 4 commits April 15, 2026 17:43
use e.isValid() instead of loc checks

Co-authored-by: Dennis <dkorpel@users.noreply.github.com>
expression.h also needs to reflect changes to CommaExp in expression.d.
Declaration statements with valid locations (like int y = x * x) should get coverage counters. Parameter copies already have Loc.initial so they're filtered by the isValid() check.
@dkorpel dkorpel dismissed their stale review April 15, 2026 13:40

Addressed

Comment thread compiler/src/dmd/inline.d
@suyashkumar102
Copy link
Copy Markdown
Contributor Author

All CI checks are passing now and review feedback has been addressed. @dkorpel , @ibuclaw — is there anything else needed before this can be merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Coverage always report 0000000 for inlined function

4 participants